Skip to content

Soft error whisper.#22475

Merged
Narsil merged 2 commits into
mainfrom
soft_error_whisper
Apr 4, 2023
Merged

Soft error whisper.#22475
Narsil merged 2 commits into
mainfrom
soft_error_whisper

Conversation

@Narsil

@Narsil Narsil commented Mar 30, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

Moving the hard error on whisper timestamp to a soft error.

Results are definitely odd potentially non sensical, but at least we're not hard errorring out.

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Fix #22053 #22053

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@Narsil Narsil requested a review from ArthurZucker March 30, 2023 14:32
@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Mar 30, 2023

Copy link
Copy Markdown

The documentation is not available anymore as the PR was closed or merged.

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks for this fix

Comment on lines 881 to 883
"There was an error while processing timestamps, we haven't found a timestamp as last token. Was"
" WhisperTimeStampLogitsProcessor used?"
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you suspect that the model was probably not finetuned with timestamps we can also add this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hesitated to put it here. While I have 1 example where I think it's true, I don't think it's enough to make it the first and foremost recommendation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I occasionally hit this error when using the pre-trained model in Flax, so it's not specific to fine-tuned ones!)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting to know !

@Narsil Narsil merged commit a515d0a into main Apr 4, 2023
@Narsil Narsil deleted the soft_error_whisper branch April 4, 2023 14:22
raghavanone pushed a commit to raghavanone/transformers that referenced this pull request Apr 5, 2023
* Soft error whisper.

* Fix format.

---------

Co-authored-by: Ubuntu <ubuntu@ip-172-31-34-94.taildb5d.ts.net>
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
* Soft error whisper.

* Fix format.

---------

Co-authored-by: Ubuntu <ubuntu@ip-172-31-34-94.taildb5d.ts.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WhisperTimeStampLogitsProcessor error while using Whisper pipelines. Was WhisperTimeStampLogitsProcessor used?

4 participants